test(desktop): story-cover the renderer crash fallback - #3947
Conversation
liuxiaocs7
changed the title
The top-level renderer ErrorBoundary is the surface hardest to reach in normal use and the one whose correctness matters most when reached: a crash the user cannot read or copy is a crash we never hear about. It had no Storybook coverage, so its four copyState values and repeat-error path could only be seen by actually crashing the app (and the failed-copy state only by additionally failing the clipboard bridge). Extract the fallback face into a pure presentational ErrorBoundaryFallback component (the class still owns all state and side effects) so each state renders directly, then add one story per state — default fallback, copy pending, copied, copy failed, and repeat error — following the FIDELITY conventions in apps/desktop/stories. Rendering the fallback directly (no real throw) keeps the render smoke free of the console.error that componentDidCatch would otherwise emit. Ran biome lint, tsc (stories and renderer), build-storybook, and the storybook render smoke (200 stories, all five error-boundary stories). Generated-by: Claude Code
82fc14b to
3e1ba88
Compare
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for adding visual coverage for the renderer crash fallback. Extracting and rendering the production ErrorBoundaryFallback is the right seam, and the idle, pending, copied, and failed visual states can provide useful regression coverage.
I am not approving the current scope yet, because two parts add claims without demonstrating the corresponding behavior. RepeatError is another static idle-state rendering rather than a reset-and-rethrow test, and the diagnostic fixtures describe a “Real path” using Maka components and files that do not exist.
I left both points inline. The smallest coherent version of this PR would keep the four real visual states, remove RepeatError unless it becomes a genuine interaction test, and label the stack data as synthetic rather than inventing a product call chain. That would retain the useful coverage while removing the low-value representations.
Reviewed with Codex and an independent @Reviewer agent. I verified the exact head, production fallback ownership, Story states, referenced renderer paths, and passing Storybook smoke coverage.
中文对照
谢谢你为 renderer 崩溃 fallback 增加视觉覆盖。提取并直接渲染生产中的 ErrorBoundaryFallback 是正确的接缝；idle、pending、copied 和 failed 四种视觉状态也能提供有价值的回归覆盖。
目前我还不准备 Approve，因为其中两部分只增加了描述，却没有证明相应行为。RepeatError 只是另一个静态 idle 状态，并没有测试 reset 后再次抛错；诊断 fixture 所谓的 “Real path” 又使用了仓库中不存在的 Maka 组件和文件。
我已经把两点留在行内。最小且连贯的版本应该保留四个真实视觉状态；除非 RepeatError 改成真正的交互测试，否则可以删除；stack 数据也应明确标为 synthetic，而不是虚构产品调用链。这样既能保留有价值的覆盖，也能删除低价值表示。
本次审查使用了 Codex 和一个独立的 @Reviewer 子代理；我核对了精确 head、生产 fallback 的职责、Story 状态、引用的 renderer 路径和 Storybook smoke 结果。
|
Thanks for the careful review, @Astro-Han. Addressed both [P2] points in 472ebae:
Re-ran locally: biome lint, tsc (stories + renderer), build-storybook, and the render smoke (199 stories, all four error-boundary states pass). Ready for another look when you have a moment. |
2 participants
Summary
The top-level renderer
ErrorBoundary(apps/desktop/src/renderer/error-boundary.tsx) had no Storybook coverage — it is the surface hardest to reach in normal use and the one whose correctness matters most when reached, so its fourcopyStatevalues and the repeat-error path could only be seen by actually crashing the app.This PR extracts the fallback face into a pure presentational
ErrorBoundaryFallbackcomponent (the class still owns all state and side effects, so behavior is unchanged), then adds one story per state — default fallback, copy pending, copied, copy failed, repeat error — each with a// Real path:annotation perstories/FIDELITY.md. Rendering the fallback directly rather than triggering a real throw keeps the render smoke free of theconsole.errorthatcomponentDidCatchwould otherwise emit.Fixes #3943
Verification
Ran locally, all passing:
biome lint— cleantsc(tsconfig.storybook.json+tsconfig.renderer.json)npm run build-storybook— all fiveproduct-shell-error-boundary--*in the indexnpm run smoke:storybook—Storybook render smoke passed (200 stories)., all five new stories render cleanThe five states are visible in Storybook under Product/Shell/Error Boundary; the render smoke above is the command-output evidence that each mounts without runtime, console, or AX errors.
AI use
Select exactly one:
Tool(s) and scope: Claude Code assisted with the
ErrorBoundaryFallbackextraction and the story authoring; the design decisions and verification are my own. The affected commit carries aGenerated-by: Claude Codetrailer.Checklist
Does this PR entail a change in behavior?